-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: replace sample_autogen code with variadic function template #13928
base: main
Are you sure you want to change the base?
Conversation
okay, so this technically works. The build failure on MSVC looks like a compiler bug to me because it works on MSVC v19.30 (VS17) but not 19.29 (VS16). |
Do you know hwere the gain for faster execution is?
Are you sure about this? From my experience templates solw down the build. |
Not entirely, I didn't bother digging into differences in the assembly but I did run the benchmarks to verify this didn't cause performance regressions Benchmark Results
Old:
New:
So I've just made some benchmark and I found that the compilation times are very similar for in the low parameter count case. It gets worse exponentally worse on GCC for more parameters, but increases linearly on clang. It may be worth experimenting with removing the |
This replaces the ginormous autogenerated file a few function templates that do the same (sometimes even faster). The primary goal was to make the header smaller to potentially speedup builds (as$N = 1, 2, 3$ ) are used in practice so I'm also fine if we just strip everything else from the file. Apart from that the function templates (while a bit difficult to read for sure) so anything the autogenerated code did while being very slightly faster as well.
sample.h
which includessample_autogen.h
is included very often) by reducing the header size. In reality, onlycopyN*
(whereBuilds on my end (and can likely be made to build everywhere).
I would appreciate some opinion on how we want to proceed. I definitely want to throw out the functions we currently don't need and I'm open to whether we want to replace them with the variadic function template or not.